Skip to content

Add validations for storage pickers #5619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Jul 19, 2025

Conversation

DinahK-2SO
Copy link
Contributor

@DinahK-2SO DinahK-2SO commented Jul 15, 2025

Description

This pull request introduces several enhancements to the StoragePickers interop functionality, focusing on validation improvements, refactoring of file type management, and updates to the PickerLocationId enumeration. The most significant changes include adding validation methods for picker properties, introducing new classes (FileTypeFilterVector and FileTypeChoicesMap) to handle file type filtering with validation, and removing obsolete entries from the PickerLocationId enumeration.

Below validations for storage pickers are added:

Error Code Message
IDS_APIERROR_INVALIDVIEWMODEVALUE The specified 'ViewMode' is invalid.
IDS_APIERROR_INVALIDSUGGESTEDSTARTLOCATIONVALUE The specified 'SuggestedStartLocation' is invalid.
IDS_APIERROR_IMPROPERFILEEXTENSION File extensions must begin with '.' and contain no wildcards.
IDS_APIERROR_STRINGSNOEMBEDDEDNULLS Strings cannot have embedded nulls.
IDS_APIERROR_MAXSAVEFILELENGTHEXCEEDED The suggested file name exceeds the maximum number of characters allowed.

Their resources can be seen in #5607

Details

Validation Enhancements

  • Added validation methods (PickerCommon::ValidateViewMode, PickerCommon::ValidateStringNoEmbeddedNulls, PickerCommon::ValidateSuggestedStartLocation, and PickerCommon::ValidateSuggestedFileName) to ensure proper input values for picker properties across FileOpenPicker, FileSavePicker, and FolderPicker. These validations prevent invalid or malformed data from being set. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

File Type Management Refactor

  • Introduced FileTypeFilterVector and FileTypeChoicesMap classes to centralize file type filtering logic with validation. These classes ensure all file type elements are validated before being added or updated. [1] [2] [3] [4]
  • Refactored FileOpenPicker and FileSavePicker constructors to utilize these new classes for managing file type filters. [1] [2]

PickerLocationId Updates

  • Removed obsolete entries (HomeGroup) from the PickerLocationId enumeration and reassigned explicit values to remaining entries for consistency. [1] [2]

Additional Improvements

  • Updated includes in several files to ensure dependencies are properly managed and to support new functionality. [1] [2] [3]

Tests

Adding new tests:

  • VerifyValidateViewMode
  • VerifyValidateSuggestedStartLocation
  • VerifyValidateSuggestedFileName
  • VerifyFolderPicker_ValidateCommitButtonText
  • VerifyFileOpenPicker_ValidateCommitButtonText
  • VerifyFileSavePicker_ValidateCommitButtonText
  • VerifyFolderPicker_ValidateSettingsIdentifer
  • VerifyFileOpenPicker_ValidateSettingsIdentifier
  • VerifyFileSavePicker_ValidateSettingsIdentifier
  • VerifyValidateSingleFileTypeFilterElement
  • VerifyValidateFileTypeFilter
  • VerifyValidateFileTypeChoices

Also verified with existing StoragePickersTests
image

@DinahK-2SO DinahK-2SO force-pushed the user/DinahK-2SO/StoragePickers_errorHandling branch from bc87f54 to a18c73b Compare July 15, 2025 04:18
@DinahK-2SO
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DinahK-2SO
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@codendone codendone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the change looks great. A few small comments.

@@ -33,6 +33,7 @@ namespace winrt::Microsoft::Windows::Storage::Pickers::implementation
}
void FileSavePicker::SettingsIdentifier(hstring const& value)
{
PickerCommon::ValidateStringNoEmbeddedNulls(value);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we care if there are embedded nulls? Same question for the Commit button text.

@DinahK-2SO
Copy link
Contributor Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@DinahK-2SO DinahK-2SO merged commit 8b1ae56 into main Jul 19, 2025
34 checks passed
@DinahK-2SO DinahK-2SO deleted the user/DinahK-2SO/StoragePickers_errorHandling branch July 19, 2025 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants